Skip to content

use same route for login_path and check_path #6143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

use same route for login_path and check_path #6143

wants to merge 3 commits into from

Conversation

snoek09
Copy link

@snoek09 snoek09 commented Jan 13, 2016

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets #6126

@xabbuh
Copy link
Member

xabbuh commented Jan 15, 2016

I think we should be more consistent throughout the documentation. Sometimes we use routes in the security configuration, sometimes we use paths. And then we should also always use the same route name for the login path route imo.

@snoek09
Copy link
Author

snoek09 commented Jan 16, 2016

👍

@HeahDude
Copy link
Contributor

👍 for @xabbuh comment as hardcoded paths mess with app_dev.php based urls and urlGenerator

@snoek09 snoek09 closed this Jan 19, 2016
@snoek09 snoek09 deleted the use-same-route-for-check-path-and-login-path branch January 19, 2016 20:45
@xabbuh
Copy link
Member

xabbuh commented Jan 19, 2016

@snoek09 Why did you close here?

@snoek09 snoek09 restored the use-same-route-for-check-path-and-login-path branch January 19, 2016 21:19
@snoek09 snoek09 reopened this Jan 19, 2016
@snoek09
Copy link
Author

snoek09 commented Jan 31, 2016

@xabbuh please let me know if this OK or not.

wouterj
wouterj reviewed Feb 6, 2016
View reviewed changes
cookbook/security/form_login_setup.rst
@@ -472,14 +449,14 @@ for the login page:
),
),

3. Be Sure /login_check Is Behind a Firewall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3. Be Sure check_path Is Behind a Firewall
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ough, this will be hard to learn to new users: /login has to be not secured, but behind a firewall... However, I think we cannot do anything about it.

@wouterj
Copy link
Member

wouterj commented Feb 6, 2016

👍 (my las tcomment can be fixed while merging)

xabbuh added a commit that referenced this pull request Feb 7, 2016
@xabbuh
[#6143] some tweaks
d46b458
@xabbuh
Copy link
Member

xabbuh commented Feb 7, 2016

Thanks @snoek09. I have merged your changes into the 2.3 branch and made some minor tweaks in d46b458.

@xabbuh xabbuh closed this Feb 7, 2016
@snoek09
Copy link
Author

snoek09 commented Feb 7, 2016

Thank you @xabbuh.

@snoek09 snoek09 deleted the use-same-route-for-check-path-and-login-path branch February 7, 2016 11:22
@weaverryan
Copy link
Member

This rocks my socks - thanks @snoek09!

@wouterj wouterj mentioned this pull request Jun 21, 2016
Merged
javiereguiluz added a commit to symfony/demo that referenced this pull request Jun 23, 2016
@javiereguiluz
feature #354 Use same URL for login form and check (WouterJ)
3dd9998 
This PR was merged into the master branch.

Discussion
----------

Use same URL for login form and check

The Security Form login listener will only intercept the request when it's a POST request, otherwise the controller will be executed. Using the same URL makes things a bit easier, removes the need for the weird controller (which is visited, but should not be executed) and makes things similar to other forms.

Related doc tickets:
 * symfony/symfony-docs#6126
 * symfony/symfony-docs#6143

Commits
-------

b7705cb Use same URL for login form and check
sayjun0505 added a commit to sayjun0505/sym_proj that referenced this pull request Apr 16, 2023
@sayjun0505
feature #354 Use same URL for login form and check (WouterJ)
28ad6cd 
This PR was merged into the master branch.

Discussion
----------

Use same URL for login form and check

The Security Form login listener will only intercept the request when it's a POST request, otherwise the controller will be executed. Using the same URL makes things a bit easier, removes the need for the weird controller (which is visited, but should not be executed) and makes things similar to other forms.

Related doc tickets:
 * symfony/symfony-docs#6126
 * symfony/symfony-docs#6143

Commits
-------

b7705cb Use same URL for login form and check
spider-yamet added a commit to spider-yamet/sym_proj that referenced this pull request Apr 16, 2023
@spider-yamet
feature #354 Use same URL for login form and check (WouterJ)
ad02c0d 
This PR was merged into the master branch.

Discussion
----------

Use same URL for login form and check

The Security Form login listener will only intercept the request when it's a POST request, otherwise the controller will be executed. Using the same URL makes things a bit easier, removes the need for the weird controller (which is visited, but should not be executed) and makes things similar to other forms.

Related doc tickets:
 * symfony/symfony-docs#6126
 * symfony/symfony-docs#6143

Commits
-------

b7705cb Use same URL for login form and check
frederickboyd added a commit to frederickboyd/frederickboyd that referenced this pull request May 25, 2025
@frederickboyd
feature #354 Use same URL for login form and check (WouterJ)
e443511 
This PR was merged into the master branch.

Discussion
----------

Use same URL for login form and check

The Security Form login listener will only intercept the request when it's a POST request, otherwise the controller will be executed. Using the same URL makes things a bit easier, removes the need for the weird controller (which is visited, but should not be executed) and makes things similar to other forms.

Related doc tickets:
 * symfony/symfony-docs#6126
 * symfony/symfony-docs#6143

Commits
-------

b7705cb Use same URL for login form and check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@snoek09 @xabbuh @HeahDude @wouterj @weaverryan